Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass OAuth 2 errors through to the app as flash[:google_sign_in_error] #31

Merged
merged 2 commits into from
Mar 6, 2019

Conversation

jeremy
Copy link
Member

@jeremy jeremy commented Mar 6, 2019

Rather than responding with 400 Bad Request due to missing code param.

Changes for apps to be aware of:

  1. Apps may receive a callback with flash[:google_sign_in_error] set to one of the OAuth 2 Authorization Code Grant errors.
  2. Mismatched state params are now passed to the app as errors so they can be treated as failed sign-ins.

@jeremy jeremy requested a review from georgeclaghorn March 6, 2019 01:15
unauthorized_client
unsupported_grant_type
invalid_scope
]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could stash these away in e.g. lib/google_sign_in/errors.rb. Exposing so apps can also validate params[:error].presence_in(GoogleSignIn::OAUTH2_ERRORS)

end

def error_message
params[:error].presence_in(GoogleSignIn::OAUTH2_ERRORS) || "invalid_request"
end
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fall back to the invalid_request catch-all.

jeremy added 2 commits March 5, 2019 18:58
Rather than responding with 400 Bad Request due to missing `code` param.
* Allows app to treat this case as a failed sign-in
* Generic `invalid_request` error in flash[:google_sign_in_error]
  rather returning 422 Unprocessable Entity directly
@jeremy jeremy merged commit 4c6b6dd into master Mar 6, 2019
@jeremy jeremy deleted the error-handling branch March 6, 2019 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants